Conversation
gr5
left a comment
There was a problem hiding this comment.
I don't know the proper syntax for signals and slots. I just create them using the QT gui and they just work for me so far. Did some for my new autoinvert dialog recently.
But I approve these changes because I looked it over and you are only changing a few things and they can be tested when we get the release.
I plan to do more than normal testing on the next release.
|
I need to add, the changes are not only small but I reverted to original code from before my changes in connection syntax. Copied from git history. If you see any warning log during your extended tests, let me know. |
|
Julien, I think we should go ahead and merge these PRs and make a new release. Or at least this one. |
|
@gr5 I think they can all be merged. For me everything is OK. I'm at work and will not have too much time. Feel free to merge any of the open PR yourself |
see #249. Compass was broken.
I was confident that the new style connect is wonderful and everything is checked at compile time. This is only partially true.
While the signal/slot existence is checked at compile time you can still fail at runtime because of what could be ABI difference between library and main application.
So for the moment, even if I'm not happy with this fix (not explaining/fixing the root cause) I propose to revert to old style to get the application to work.
When testing if you see any missed connect in logs, please inform me so that I revert them too.